Skip to content

fix: intermittent media 401 errors after token refresh or SW restart#548

Open
Just-Insane wants to merge 18 commits intoSableClient:devfrom
Just-Insane:fix/media-error-handling
Open

fix: intermittent media 401 errors after token refresh or SW restart#548
Just-Insane wants to merge 18 commits intoSableClient:devfrom
Just-Insane:fix/media-error-handling

Conversation

@Just-Insane
Copy link
Copy Markdown
Contributor

@Just-Insane Just-Insane commented Mar 26, 2026

Description

Fixes a chain of service worker session bugs that caused intermittent 401/404 errors on authenticated media:

  • Media requests no longer return 401 after a token refresh or SW restart
  • Session cache is now guaranteed to flush before the SW can be killed, fixing intermittent 401s on iOS
  • OIDC token refreshes are forwarded to the service worker as they happen
  • Fixed unhandled promise rejections from media decrypt failures

Fixes #

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

AI disclosure:

  • Partially AI assisted (GitHub Copilot)

sw.ts: The setSession/clearSession cache writes are now wrapped in event.waitUntil so the browser keeps the SW alive until the caches.put resolves — preventing stale tokens surviving a mid-write SW kill on iOS. Added a requestSessionWithTimeout fallback in fetchMediaWithRetry for the window between a token refresh completing and the resulting setSession message arriving. index.tsx: Calls pushSessionToSW synchronously before React renders so the SW has a valid token before the first fetch event. ClientRoot.tsx: Wires pushSessionToSW into the SDK's onTokenRefresh callback so OIDC token refreshes propagate to the SW immediately. Decrypt failure and media error async paths now have proper rejection handling to avoid unhandled promise rejections.

useAsync re-threw errors after storing them in Error state. Any call site
that discards the returned Promise (e.g. fire-and-forget useEffect calls
like loadSrc() or loadThumbSrc()) produced an 'Uncaught (in promise)'
console error — notably 'Mismatched SHA-256 digest' from encrypted media
decryption failures.

The error is already fully handled: it is stored in AsyncStatus.Error state
and components surface it via a retry button. The re-throw added no value
and only caused noise / unhandled rejection warnings.

Update the test accordingly — the .catch(() => {}) guard was only needed to
silence the re-throw and is no longer necessary.
Stores refresh_token from the login response, passes it and a
tokenRefreshFunction to createClient so the SDK can auto-refresh
expired access tokens. The callback propagates new tokens to both
the sessionsAtom (localStorage) and the service worker via
pushSessionToSW, preventing 401 errors on authenticated media
after token expiry.
@Just-Insane Just-Insane force-pushed the fix/media-error-handling branch from 8ecb7cc to 715e9e4 Compare March 26, 2026 03:36
@Just-Insane Just-Insane marked this pull request as ready for review March 26, 2026 03:40
@Just-Insane Just-Insane requested review from 7w1 and hazre as code owners March 26, 2026 03:40
@Just-Insane Just-Insane changed the title fix: suppress unhandled promise rejections and wire OIDC token refresh to service worker fix: OIDC/SSO Token + Media issues Mar 26, 2026
@Just-Insane Just-Insane marked this pull request as draft March 26, 2026 13:17
There is a timing window between when the SDK refreshes its access
token (tokenRefreshFunction resolves and pushSessionToSW is called)
and when the resulting setSession postMessage is processed by the SW.
Media requests that land in this window carry the stale token and
receive 401. The browser then retries those image/video loads, hitting
the SW again with the same stale token — producing the repeated 401
bursts visible in the console.

fetchMediaWithRetry() resolves this by retrying once on 401: it
re-checks the in-memory sessions map (and preloadedSession fallback)
for a different access token. By the time the retry runs, setSession
will normally have been processed and the map will hold the new token.
Applied consistently across all four branches of the fetch handler.
@Just-Insane Just-Insane force-pushed the fix/media-error-handling branch from ae8c97d to c177218 Compare March 26, 2026 13:46
@Just-Insane Just-Insane marked this pull request as ready for review March 26, 2026 14:08
When preloadedSession holds a stale token and the live setSession from
the page hasn't arrived yet, fetchMediaWithRetry had no fresher token
to retry with and immediately returned the 401 response.

Add a second-chance retry: if in-memory token lookup yields nothing
better, call requestSessionWithTimeout(1500ms) to ask the live client
tab for its current session, then retry with it if the token differs.

This fixes the startup race where thumbnails (message sender avatars,
URL preview images, etc.) 401 briefly before the page pushes its
fresh session to the service worker.
Copilot AI review requested due to automatic review settings March 27, 2026 14:09
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR targets media reliability by addressing token refresh propagation and service-worker handling of authenticated media fetches, plus reducing noisy “Uncaught (in promise)” console errors in media-loading UI flows.

Changes:

  • Adjust useAsyncCallback error handling behavior and update its tests.
  • Persist OIDC refresh_token/expires_in_ms into session state and wire a tokenRefreshFunction into the Matrix client, propagating refreshed access tokens to the service worker.
  • Add a SW media fetch wrapper that retries once on HTTP 401 using the latest in-memory/live session token.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/sw.ts Adds fetchMediaWithRetry() and routes media fetch paths through it to mitigate 401s during token-refresh propagation races.
src/client/initMatrix.ts Passes refresh-token options into createClient and plumbs an optional onTokenRefresh callback to propagate new tokens outward.
src/app/pages/client/ClientRoot.tsx Supplies the onTokenRefresh callback to update sessions state and push updated tokens to the SW.
src/app/pages/auth/login/loginUtil.ts Stores refresh_token and expires_in_ms into the session record on login.
src/app/hooks/useAsyncCallback.ts Changes error handling in the async wrapper (no longer rejects on error) and documents rationale.
src/app/hooks/useAsyncCallback.test.tsx Updates error test to no longer expect a rejected promise.
.changeset/fix-media-error-handling.md Adds a patch changeset describing the combined fixes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Just-Insane Just-Insane marked this pull request as draft March 27, 2026 15:03
- useAsyncCallback: re-throw in catch to preserve rejection semantics;
  add no-op .catch wrapper in useAsyncCallback to suppress unhandled
  rejection warnings for fire-and-forget callers
- useAsyncCallback.test: expect rejects.toBe(boom) to assert rejection
- loginUtil: use != null guards for refresh_token / expires_in_ms to
  correctly handle zero/falsy-but-present values
- initMatrix: use typeof === 'number' guard for expires_in_ms expiry calc
- ClientRoot: pass activeSession.userId to both pushSessionToSW calls
  so the SW session record keeps a complete userId
Just-Insane added a commit to Just-Insane/Sable that referenced this pull request Mar 27, 2026
- useAsyncCallback: re-throw in catch to preserve rejection semantics;
  add no-op .catch wrapper in useAsyncCallback to suppress unhandled
  rejection warnings for fire-and-forget callers
- useAsyncCallback.test: expect rejects.toBe(boom) to assert rejection
- loginUtil: use != null guards for refresh_token / expires_in_ms to
  correctly handle zero/falsy-but-present values
- initMatrix: use typeof === 'number' guard for expires_in_ms expiry calc
- ClientRoot: pass activeSession.userId to both pushSessionToSW calls
  so the SW session record keeps a complete userId
Resolve stash conflict in DevelopTools.tsx — keep the improved Rotate
Encryption Sessions description and success message text from the
stashed changes.

Add an immediate synchronous sendSessionToSW() call in index.tsx before
React mounts. When the SW is already active (normal page reload),
navigator.serviceWorker.controller is set and the postMessage is sent
before createRoot().render() runs and before any <img> elements fire
fetch events. This discards the stale preloadedSession in the SW (which
could be from a previous token that is no longer current) before the
first thumbnail/media fetches arrive, preventing the race condition that
produced 401 errors on initial load with Retry buttons.
@Just-Insane Just-Insane changed the title fix: OIDC/SSO Token + Media issues fix: complete SW session fix — OIDC token refresh, media 401 retry, and initial-load race Mar 28, 2026
persistSession was called fire-and-forget inside setSession. If the
browser killed the service worker between the setSession message being
processed and the caches.put call completing, the persisted session cache
would retain the previous (stale) token. On the next SW restart, the
activate handler loads this stale session into preloadedSession, causing
media requests to 401 until the retry path eventually obtains a fresh
token from the live page via requestSessionWithTimeout.

Using event.waitUntil on the persist/clear operation ensures the browser
keeps the SW alive until the cache write resolves, eliminating the race
window that could leave the cache in an inconsistent state.
@Just-Insane Just-Insane changed the title fix: complete SW session fix — OIDC token refresh, media 401 retry, and initial-load race fix: SW session persistence race, media 401 retry, OIDC token refresh wiring, and initial media load race Mar 28, 2026
@Just-Insane Just-Insane changed the title fix: SW session persistence race, media 401 retry, OIDC token refresh wiring, and initial media load race fix: intermittent media 401 errors after token refresh or SW restart Mar 28, 2026
The reload path that fires when the active session user changes was
calling pushSessionToSW without the userId argument, which would clear
the stored userId in the service worker. Pass activeSession.userId
consistently, matching the initial-load and token-refresh call sites.
@Just-Insane Just-Insane marked this pull request as ready for review March 28, 2026 02:52
For media fetches without an immediately-available client session, we used
a by-baseUrl fallback that could pick the wrong account token when multiple
sessions share one homeserver, causing intermittent 401 responses.

Prefer client-specific resolution and only use by-baseUrl fallback when
there is exactly one matching session for the request URL.
Extend media interception coverage to include preview_url plus client v3
media endpoints so these requests also get bearer injection and 401 retry
handling.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants